Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

sensors: use interpolation to set param from RC input #7676

Closed
wants to merge 1 commit into from

Conversation

davidaroyer
Copy link
Contributor

@davidaroyer davidaroyer commented Jul 24, 2017

This PR changes the set_params_from_rc() function to use linear interpolation between the user-defined min, center, and max values based on the RC Input value. Since rc_val is essentially a percentage, and constrained from -1.0 to +1.0, all that's required to implement this is a small change to the "param_val" calculation as such:

  • if rc_val is positive (or 0.0): param_val = [center value] + rc_val * ([max value] - [center value])
  • if rc_val is negative: param_val = [center value] + rc_val * ([center value] - [min value])

Of course, this will be a change of philosophy in parameter tuning via RC input. In my personal case, while gain tuning a new airframe I set some values for MC_PITCHRATE_D assuming the parameter would interpolate between min, center, and max values. Since I left the "Scale" value to 1.0 and the derivative gains are so small, any small change in the RC Input away from the center input would immediately change the parameter from center to min or center to max. So the derivative gain didn't do anything as I increased the transmitter knob from min to its center, then just past half-way on the knob, the derivative gain immediately went from my min to my max value, causing some pretty wild behavior.

All that said, in my opinion, the presented change is a simpler approach to parameter tuning via RC input such that the end user doesn't need to figure out a "Scale" value for appropriate interpolation, rather the end user only needs to set min, center, and max values and the software will set the parameter accordingly.

@LorenzMeier
Copy link
Member

Jenkins test this please.

@LorenzMeier
Copy link
Member

LorenzMeier commented Jul 27, 2017

This makes it non-linear over the full range. I'm not sure if that's more intuitive for others. If the zero value is close to min or max it will lead to very different scales to either side.

@davidaroyer
Copy link
Contributor Author

@LorenzMeier Yes, technically it's piece-wise linear, so linear from minimum-to-center, then linear from center-to-maximum values. The advantage is that the interpolation is taken care of by the software; no need to set a scale value (essentially a slope). I would argue that this is more intuitive than requiring an end-user to calculate their own scale value, especially if they're gain tuning in the field and there's no clear indication of what the scale value does (specifically from the ground station).

Below is an illustration wherein I set a range of values to tweak MC_PITCHRATE_D. Without knowing that "Scale" needs to be adjusted - QGC says "Scale (keep default)" - the only possible param values from the transmitter knob are essentially the min, center, and max values. Full disclosure, in this particular case, you do have some in-between values here, but as soon as rc_val >= +/-0.02, the param is at min/max.

image

Of course, this isn't quite as bad if you're tuning with some larger numbers (e.g. MC_PITCHRATE_P -
see below), but even so, the change in this PR gives a more intuitive range of values between min, center, and max.

image

Indeed, you may have quite different scales on either side of the center (zero) value. However, this makes plenty of sense if you want to start tuning from the middle and perhaps you want to severely limit your tuning in one direction and leave a large range in the opposite direction.

@bkueng
Copy link
Member

bkueng commented Jul 28, 2017

I have nothing against the change, but it will require changing the UI, in particular this is how QGC displays it:
qgc_rc_to_param

This patch removes the scale.
What we could do is adjust the min/max values of the UI to center -/+ scale when the user changes the scale. Or displaying a graph as the one you show would make it clear as well.

@stale
Copy link

stale bot commented Jan 20, 2019

This issue has been automatically marked as stale because it has not had recent activity. It will be closed if no further activity occurs. Thank you for your contributions.

@dagar
Copy link
Member

dagar commented Feb 5, 2019

Still interested in this?

@stale
Copy link

stale bot commented Jul 10, 2019

This issue has been automatically marked as stale because it has not had recent activity. It will be closed if no further activity occurs. Thank you for your contributions.

@stale
Copy link

stale bot commented Jul 24, 2019

Closing as stale.

@stale stale bot closed this Jul 24, 2019
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants